ci, make: "lint" (flake8 & mypy) replace "quality" #2502
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I know I did that, so I'm owning my mistake, and rename "make quality" by "make lint". A linter is something that checks your code for errors, and both flake8 and mypy are linters. There are now 2 new make commands: * make lint-style: perform any code style check (today: flake8 only), it could be used for a pylint check for example, or isort, or rst check, or anything that could help us with code style in the future * make lint-type: perform any type check (today: mypy only) The command `quality` is replaced by the command `lint`, which runs both `lint-style` and `lint-type`. Type check is now mandatory in CI, and the PR template has been updated accordingly.
SnoopJ
approved these changes
Aug 12, 2023
There was a problem hiding this comment.
LGTM. If you want to also include an update to the contributed tox config, here's a diff for that:
click for diff
diff --git a/contrib/tox.ini b/contrib/tox.ini
index 393de027..391d8b11 100644
--- a/contrib/tox.ini
+++ b/contrib/tox.ini
@@ -6,8 +6,7 @@ envlist =
skip_missing_interpreters = true
ignore_base_python_conflict = true
labels =
- mypy = py{37,38,39,310,311}-mypy
- quality = py{37,38,39,310,311}-quality
+ lint = py{37,38,39,310,311}-lint
test = py{37,38,39,310,311}-test
@@ -39,8 +38,7 @@ setenv =
commands =
qa: make -C.. qa
- mypy: make -C.. mypy
- quality: make -C.. quality
+ lint: make -C.. lint
test: make -C.. test(Upon review, I don't think it's worth splitting the two forms of linting for the sake of tox, so I just collapsed them into one label)
I approve of the new naming 😁
|
Let's go then. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
I think Sopel 8.0 is ready to enforce type check (close #2461).
I know I did that, so I'm owning my mistake, and rename "make quality" by "make lint". A linter is something that checks your code for errors, and both flake8 and mypy are linters.
There are now 2 new make commands:
The command
qualityis replaced by the commandlint, which runs bothlint-styleandlint-type.Type check is now mandatory in CI, and the PR template has been updated accordingly.
Note
I used the kebab-case style for the lint sub-commands, while other sub-commands use the snake_case. We can either switch all to kebab-case (my preferred style for command), or I keep it to snake_case. I have a preferred option, however for the sake of consistency, I'll be happy to revert that if there is an objection.
Checklist
make qa(runsmake lintandmake test)